Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace typescript #60

Merged
merged 34 commits into from
Oct 28, 2022
Merged

Replace typescript #60

merged 34 commits into from
Oct 28, 2022

Conversation

segayuu
Copy link
Contributor

@segayuu segayuu commented Sep 26, 2019

One of the reasons why warehouse is hardly maintained is that it doesn't understand the type.
However, the warehouse package is too complex to be typed with DefinitelyTyped, and it is quite possible to make typing errors.

@segayuu segayuu requested a review from a team September 26, 2019 08:17
@segayuu segayuu mentioned this pull request Sep 26, 2019
@yoshinorin
Copy link
Member

yoshinorin commented Sep 26, 2019

@segayuu
IMHO we should release warehouse v3.0 and upgrade hexo's dependency v3.0 before migrate to TypeScript.

@curbengh
Copy link
Contributor

I just added warehouse@3 as part of hexo v4 milestone.

@yoshinorin
Copy link
Member

@segayuu
What point of view should we check? Just migrate js to ts?
For example, some instance member can specify private. Should we review such code?

@segayuu
Copy link
Contributor Author

segayuu commented Sep 29, 2019

The purpose of this PR is to set noImplicityAny to true, and strict typing from that achievement is an arbitrary position. If the private attribute is added, it is necessary to modify the test file implementation.

@YuJianghao
Copy link

Do we have updates on this pr?

For now I think that hexo-warehouse is the best json db for tiny apps :p
Waiting for ts updates.

@tmirun
Copy link

tmirun commented Dec 31, 2020

I think the same @YuJianghao

@yoshinorin
Copy link
Member

yoshinorin commented Jun 22, 2022

@hexojs/core
Now all tests are green.

const Promise = require('bluebird');
const sinon = require('sinon');
const fs = Promise.promisifyAll(require('fs'));
// @ts-nocheck
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add @ts-nocheck to all test files.
This workaround is temporarily. I will update the test code.

@yoshinorin
Copy link
Member

@hexojs/core
Now ready for review. Does someone would you please review?

I think we can improve our code more type safely and robustly. However, I think we need more time to do it. Also, I will fix if the current PR has a critical problem. But, IMHO I want to merge this PR once and improve our code step by step.

Thank you :)

@yoshinorin yoshinorin requested a review from a team July 23, 2022 21:33
package.json Outdated
},
"engines": {
"node": ">=14"
"node": ">=14 <18.5"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a workaround.
The Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close error occurs with Node 18.6. I don't know this problem is a bug or a spec change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO we should delete <18.5 before release the new version (5.0.0)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted

@yoshinorin
Copy link
Member

Updated: Merge the master branch to this PR branch.

src/database.ts Outdated Show resolved Hide resolved
@yoshinorin
Copy link
Member

@hexojs/core
Can we merge this? or need more review?

@yoshinorin yoshinorin merged commit 44394e2 into hexojs:master Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants